-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not yet familiar enough with oauth2_proxy guts to review most of this, but had one doc-related comment above!
README.md
Outdated
2. Setup oauth2_proxy with the correct provider and using the default ports and callbacks. | ||
3. Login with the fixture use in the dex guide. | ||
|
||
-provider oidc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these be more explicitly marked as oauth2_proxy
args? It makes sense if you've played with oauth2_proxy before, but may help those who are early in their evaluation/have yet to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README section was just copied over from the previous PR. Let me revamp it a bit. Thanks.
@@ -137,6 +141,22 @@ func (o *Options) Validate() error { | |||
msgs = append(msgs, "missing setting for email validation: email-domain or authenticated-emails-file required.\n use email-domain=* to authorize all email addresses") | |||
} | |||
|
|||
if o.OIDCIssuerURL != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops, needs to come after overriding http.DefaultClient.
@jehiah any chance someone at bitly has bandwidth to chime in on this PR? |
@ericchiang I tried to use the oidc provider with dex, but without success.
I patched dex to see what is in
Do you have any idea why? BTW: -redeem-url seems to be ingnored |
@mattenklicker --redeem-url is implied from discovery metadata. There aren't spec compliant reasons to override it. The error your seeing implies that the URL that the bitly proxy is sending as a redirect URI isn't registered with dex. This is the same issue you'd have with google, github, etc. if you didn't register the correct redirect. What flags are you running with and what does your |
I don't actually see the redirect URL being set in the |
@bison the oauth2.Config is just used for token exchange. The Redeem URL reuses the proxy's built in OAuth2 redirects and is set here: https://github.com/bitly/oauth2_proxy/pull/389/files#diff-4aae454123f50b906f9aaef44f10011eR154 |
@ericchiang right, but the the |
@ericchiang the auth chain is: app -> oauth2-proxy -> dex -> ldap app is a kubernetes ingress config metadata:
annotations:
ingress.kubernetes.io/auth-signin: https://$host/oauth2/sign_in
ingress.kubernetes.io/auth-url: https://$host/oauth2/auth
spec:
rules:
- host: kubernetes-dashboard....
http:
paths:
- backend:
serviceName: kubernetes-dashboard
servicePort: 80
path: / and auth redirect ingress: spec:
rules:
- host: kubernetes-dashboard....
http:
paths:
- backend:
serviceName: oauth2-proxy
servicePort: 80
path: /oauth2 oauth2-proxy config: args:
- --provider=oidc
- --redirect-url=https://oauth2-proxy..../oauth2/callback
- --email-domain=*
- --upstream=file:///dev/null
- --http-address=0.0.0.0:4180
- --cookie-secure=false
- --oidc-issuer-url=https://dex....
env:
- name: OAUTH2_PROXY_CLIENT_ID
value: oauth2_proxy
- name: OAUTH2_PROXY_CLIENT_SECRET
value: proxy
- name: OAUTH2_PROXY_COOKIE_SECRET
value: secret dex static clients config: staticClients:
- id: oauth2_proxy
secret: proxy
name: 'Oauth2-Proxy'
redirectURIs:
- 'https://oauth2-proxy..../oauth2/callback' Note: The app-callback-URL itself is nowhere registered. But I don't think that's the problem here. If it should be registered, than it should be with oauth2-proxy (?). I tried to add it to Another question: |
I'm stuck on the same invalid_request error that @mattenklicker and others have mentioned. Does anyone know of a way to get this working? |
I still think that's because the |
Yep, sure enough. This is a one-line fix, and now it works: diff --git a/providers/oidc.go b/providers/oidc.go
index 580c1e3..98d9220 100644
--- a/providers/oidc.go
+++ b/providers/oidc.go
@@ -23,6 +23,7 @@ func NewOIDCProvider(p *ProviderData) *OIDCProvider {
func (p *OIDCProvider) Redeem(redirectURL, code string) (s *SessionState, err error) {
ctx := context.Background()
c := oauth2.Config{
+ RedirectURL: redirectURL,
ClientID: p.ClientID,
ClientSecret: p.ClientSecret,
Endpoint: oauth2.Endpoint{ |
Nice. Works fine. |
As an OIDC (and OAuth) n00b, I'm a bit disoriented about where this PR stands. Can someone help me with the below questions?
|
I tried this PR with the Keycloak as the OpenIDC Provider and it works fine but when I enable |
@emmanuel I added the Authorization header because it makes oauth2_proxy work properly with the Kubernetes dashboard app, which uses the bearer token to act on the behalf of the authenticated user. It's probably not needed for most other apps. |
@benley thanks for the explanation. that sounds like a good use-case (i.e., I'm off to deploy the proxy in front of kube-dashboard 😁 ). |
providers/oidc.go
Outdated
Endpoint: oauth2.Endpoint{ | ||
TokenURL: p.RedeemURL.String(), | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this config need the RedirectURL
? I was getting a 401 without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I encountered the same as @travisofthenorth.
@ericchiang ?
72d46b9
to
ccbfde5
Compare
Comments addressed. Please let me know if there's anything else blocking this PR. |
See the README for usage with Dex or any other OIDC provider. To test run a backend: python3 -m http.server Run dex and modify the example config with the proxy callback: go get github.com/coreos/dex/cmd/dex cd $GOPATH/src/github.com/coreos/dex sed -i.bak \ 's|http://127.0.0.1:5555/callback|http://127.0.0.1:5555/oauth2/callback|g' \ examples/config-dev.yaml make ./bin/dex serve examples/config-dev.yaml Then run the oauth2_proxy oauth2_proxy \ --oidc-issuer-url http://127.0.0.1:5556/dex \ --upstream http://localhost:8000 \ --client-id example-app \ --client-secret ZXhhbXBsZS1hcHAtc2VjcmV0 \ --cookie-secret foo \ --email-domain '*' \ --http-address http://127.0.0.1:5555 \ --redirect-url http://127.0.0.1:5555/oauth2/callback \ --cookie-secure=false Login with the username/password "admin@example.com:password"
ccbfde5
to
cb48577
Compare
My feedback has been addressed 👍 |
@travisofthenorth great! Who can click merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 changes look good
🤣 |
I've almost got this working in front of kube-dashboard. I get authenticated through the proxy but the handoff of the token doesn't seem to make it to kube-dashboard... I see in the kube-dashboard 1.7 docs it mentions bearer tokens. I don't really see how they are provided in this ps though. Am I missing a flag? Any other ideas? |
Are you using a build that's patched to pass the user's raw token along in a Authorization header? Here's what finally got that part working for me: postmates@5a27234 |
I checked out trunk from earlier today. Is there still patches needing to be merged? |
yeah.... looks like that other patch is not here yet... Thanks for the pointer to it. Any plans for a pr here? |
I hadn't opened a PR for that feature because I have been assuming that it's not a feature that would be accepted upstream, since it's rather specific to one auth backend and a use case that is nonstandard. At least I think it's nonstandard...? |
I could see other things wanting to use bearer tokens too? Seems like a very minor thing to fork a project over. If upstream will take it, probably worth a shot? |
tried to cherry pick the one patch. it fails due to: Looks like the two code bases are farther apart then I had hoped. :( |
I think I figured it out. 'go get' was fetching a new checkout of the bitly/oauth2_proxy rather then reusing the existing, patched checkout. After patching the downloaded source, the patch seems to have worked as is. |
Any chance we can get this patch in the upstream? |
I would also be interested in this patch being incorporated in the official project, for the same purpose[1]. Basically, "would it hurt to include it upstream"? [1] for use with the Kubernetes dashboard, which requires the actual auth JWT to enable the dashboard to impersonate/operate-on-behalf-of the logged in user |
Hello ! Can some1 share me a container with this PR merged |
Takes over from #165
Closes #332
cc @philips @gtaylor
See the README for usage with Dex or any other OIDC provider.
To test run a backend:
Run dex and modify the example config with the oauth2 callback.
Then run the oauth2_proxy:
Login with the username/password
admin@example.com:password